-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rely on PGO for isinst/castclass #65922
Conversation
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsFollow up to #65460 Example (with using System.Runtime.CompilerServices;
using System.Threading;
public interface IClass {}
public class ClassA : IClass {}
public class ClassB : ClassA {}
public class Program
{
[MethodImpl(MethodImplOptions.NoInlining)]
static ClassA CastToClassA(object o) => (ClassA)o;
[MethodImpl(MethodImplOptions.NoInlining)]
static bool IsClassA(object o) => o is ClassA;
[MethodImpl(MethodImplOptions.NoInlining)]
static IClass CastToIClass(object o) => (IClass)o;
[MethodImpl(MethodImplOptions.NoInlining)]
static bool IsIClass(object o) => o is IClass;
public static void Main()
{
// promote methods to tier1
var b = new ClassB();
for (int i = 0; i < 100; i++)
{
CastToClassA(b);
IsClassA(b);
CastToIClass(b);
IsIClass(b);
Thread.Sleep(16);
}
}
} Codegen diff: https://www.diffchecker.com/RFblv9RB
|
@AndyAyersMS @jakobbotsch PTAL This PR consumes profile data from mibc for casts/isinst. I guarded it with Also, I added "random class" stress mode support for it |
@AndyAyersMS @jakobbotsch PTAL |
CORINFO_CLASS_HANDLE likelyCls = likelyClass.clsHandle; | ||
|
||
if ((likelyCls != NO_CLASS_HANDLE) && | ||
(likelyClass.likelihood > (UINT32)JitConfig.JitGuardedDevirtualizationChainLikelihood())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this rather get its own config variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just didn't want to produce even more variables 😄 and the default value for this one was OK to me.
in my other PR to add "multiple guesses" I slightly changed the whole logic so I'll leave it as is for now
Follow up to #65460 (and d-o)
Example (with
DOTNET_JitCastProfiling=1
):Codegen diff: https://www.diffchecker.com/RFblv9RB